[cppinterop] Upgrade to latest for cppyy migration#22728
Conversation
Test Results 19 files 19 suites 2d 17h 36m 25s ⏱️ Results for commit 11a13bb. ♻️ This comment has been updated with latest results. |
|
@aaronj0, I believe we need to also move |
Yes, I have that obvious change and more patches in the pipeline.. (adapting to the new types, etc) |
aaronj0
force-pushed
the
cppinterop-upgrade
branch
3 times, most recently
from
7618eb1 to
8f4c938
Compare
aaronj0
force-pushed
the
cppinterop-upgrade
branch
from
7fa2272 to
3427288
Compare
aaronj0
force-pushed
the
cppinterop-upgrade
branch
2 times, most recently
from
09a28ec to
4dbc826
Compare
|
I believe this upgrade now passes the CI, the windows x64 failures are visible on master. I will upstream the required patches and bump this upgrade to the latest commit. cc @guitargeek |
aaronj0
force-pushed
the
cppinterop-upgrade
branch
from
4dbc826 to
a0a483b
Compare
guitargeek
left a comment
guitargeek
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the update. Unfortunately we don't have green builds on Windows, but that problem is also present in the nightlies.
aaronj0
force-pushed
the
cppinterop-upgrade
branch
3 times, most recently
from
83e29a3 to
523fd79
Compare
The latest CppInterOp upgrade introduces opaque-handle structs Cpp::ObjectRef, Cpp::DeclRef, and Cpp::InterpRef (CppInterOpTypes.h) to replace void*/typedef pointer arguments.
The latest CppInterOp upgrade changes UseExternalInterpreter to also install a CppInteropDiagConsumer onto Cling's DiagnosticsEngine. Rootcling later wraps the current diag client with CheckModuleBuildClient via DiagnosticsEngine::setClient which deletes CppInterOp's previously-owned client. This leads to CheckModuleBuildClient holding a dangling fChild pointer. Without this patch, cling::Interpreter::ShutDown crashes. Since rootcling does not use CppInterOp, we can avoid registering the interpreter with CppInterOp if we are running in rootcling.
aaronj0
force-pushed
the
cppinterop-upgrade
branch
from
523fd79 to
11a13bb
Compare
3 participants
Test and land the latest upgrade, required to rebase the cppyy migration PR on top